Address PR #75 review feedback: env var support, priority names, and whitespace - #79
Conversation
- Make create-github-issues.py read GITHUB_REPOSITORY env var with fallback - Update tracking markdown with proper priority names (Critical, High) - Add missing P0-3 WebSocket infrastructure issue to tracking - Remove trailing whitespace from requirements.txt Co-authored-by: tim-dickey <80638631+tim-dickey@users.noreply.github.com>
Replace TBD placeholder with explanatory note about P0-3 not being created, avoiding confusion in the checklist tracking Co-authored-by: tim-dickey <80638631+tim-dickey@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates the issue-creation tooling and tracking docs to address PR #75 feedback, improving fork/CI compatibility and clarifying priority labeling in generated artifacts.
Changes:
- Read
GITHUB_REPOSITORYfrom the environment increate-github-issues.py(fallback totim-dickey/trivia-app). - Clarify P0/P1 priority names in the tracking markdown and add a note about the missing P0-3 issue.
- Remove trailing whitespace from two
backend/requirements.txtentries.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/create-github-issues.py | Adds env-var driven repo selection for gh issue create runs (supports forks/wrappers). |
| backend/requirements.txt | Removes trailing spaces on two dependency lines. |
| _bmad-output/implementation-artifacts/code-review-issues-tracking.md | Replaces “(unknown)” with “Critical/High” and documents the uncreated P0-3 item. |
| from typing import List, Dict, Tuple, Optional | ||
|
|
||
| REPO = "tim-dickey/trivia-app" | ||
| REPO = os.environ.get("GITHUB_REPOSITORY", "tim-dickey/trivia-app") |
There was a problem hiding this comment.
os.environ.get("GITHUB_REPOSITORY", ...) only falls back when the var is unset, not when it's set to an empty/whitespace value. If GITHUB_REPOSITORY is exported but empty, gh issue create --repo "" will fail. Consider normalizing (e.g., .strip()) and falling back when the value is falsy.
| REPO = os.environ.get("GITHUB_REPOSITORY", "tim-dickey/trivia-app") | |
| REPO = (os.environ.get("GITHUB_REPOSITORY") or "").strip() or "tim-dickey/trivia-app" |
There was a problem hiding this comment.
Applied the suggestion in commit fdac3c5. The code now normalizes the env var with .strip() and falls back to the default when the value is unset, empty, or whitespace-only.
Tested all scenarios:
- Valid value → uses it
- Unset → default
- Empty string → default
- Whitespace → default
|
@copilot apply changes based on the comments in this thread |
Normalize env var value with .strip() and fall back to default when empty or whitespace-only, preventing gh issue create failures with empty --repo flag Co-authored-by: tim-dickey <80638631+tim-dickey@users.noreply.github.com>
Applied the review feedback in commit fdac3c5. The |
Addresses four code review comments from PR #75 regarding issue creation scripts, tracking documentation, and code quality.
Changes
Robust environment variable handling:
create-github-issues.pynow readsGITHUB_REPOSITORYfrom environment, normalizes with.strip(), and falls back to"tim-dickey/trivia-app"when unset, empty, or whitespace-only, preventinggh issue createfailures with invalid repo valuesPriority name clarity: Updated
code-review-issues-tracking.mdto display"Critical"and"High"instead of"(unknown)"for P0/P1 sectionsComplete issue tracking: Added explanatory note documenting that P0-3 (WebSocket Infrastructure) from source JSON was not created during initial issue creation run
Trailing whitespace: Removed trailing spaces from
backend/requirements.txtlines 25-26💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.